Skip to content

Conversation

@gagath
Copy link
Contributor

@gagath gagath commented Nov 25, 2024

No description provided.

@gagath gagath requested a review from AnneCYH as a code owner November 25, 2024 10:50
@gagath gagath requested a review from smb49 November 26, 2024 08:36
In case the provenance is anything else, you should explicit the source git
tree in full::

(cherry picked from commit 622f21994506e1dac7b8e4e362c8951426e032c5 git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though we say before that non-upstream sources will rather be frowned upon. IMO not a blocker. Just mentioning it. Thinking about it maybe one other special case might be added which has happened recently. Picking or backporting from another series (which might even be SAUCE there).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a section with example for this usecase.

@smb49
Copy link

smb49 commented Nov 27, 2024

Adding this just as a generic comment. Trying to give a little background here. The BugLink location is not strictly enforced. Just over time it got established at the first line in the description. This allows finding it more easily. In the past some had it as the first line of the additional s-o-b.

On the SRU template... the required location is the bug report. And the audience is a different one from the email submission. I am really not sure how much of this is still done but in the past any package update (and the kernel in that sense is just another package) was reviewed by a SRU review team which was not the kernel team but somewhere around foundations. What gets written there should rather be high level from a user point of view. The most mis-understood part being regression potential. There were attempts to use a more clear term. But basically what was intended there was a description which explains what it would look like if this change caused a regression. So someone doing regression testing might understand where that is coming from.
The email cover letter is rather for us/the reviewers of the patches. And having the SRU justification copied in there is an easy way to introduce things without further work. But what I would envision there in addition is explaining the fixes picked in a way that help the reviewer to understand the choices made. That include the one why a SAUCE approach was taken.

Copy link

@smb49 smb49 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me this looks good regardless of the notes I added. This is an improvement to the current situation at least.

Shorten some titles to make them fit in the ToC.

Signed-off-by: Agathe Porte <[email protected]>
@gagath gagath force-pushed the gagath/patch-acceptance-criteria branch from 4758fe6 to 4fc0929 Compare November 27, 2024 10:34
After discussion with @smb some criteria are not hard requirements but
have been used since some time. Change the wording to reflect that.

Signed-off-by: Agathe Porte <[email protected]>
@gagath
Copy link
Contributor Author

gagath commented Nov 27, 2024

Adding this just as a generic comment. Trying to give a little background here. The BugLink location is not strictly enforced. Just over time it got established at the first line in the description. This allows finding it more easily. In the past some had it as the first line of the additional s-o-b.

Reformulated "must" to "should" for the BugLink to reflect that.

On the SRU template... the required location is the bug report. And the audience is a different one from the email submission. I am really not sure how much of this is still done but in the past any package update (and the kernel in that sense is just another package) was reviewed by a SRU review team which was not the kernel team but somewhere around foundations. What gets written there should rather be high level from a user point of view. The most mis-understood part being regression potential. There were attempts to use a more clear term. But basically what was intended there was a description which explains what it would look like if this change caused a regression. So someone doing regression testing might understand where that is coming from.

Reformulated "must" to "should" for the SRU template in cover letter to reflect that.

The email cover letter is rather for us/the reviewers of the patches. And having the SRU justification copied in there is an easy way to introduce things without further work. But what I would envision there in addition is explaining the fixes picked in a way that help the reviewer to understand the choices made. That include the one why a SAUCE approach was taken.

Added some extra explanation for the cover letter section.

@gagath gagath merged commit b5d529a into main Nov 27, 2024
@gagath gagath deleted the gagath/patch-acceptance-criteria branch November 27, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants